Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add subsec to ActiveSupport::TimeWithZone#inspect #38874

Merged
merged 3 commits into from
Apr 11, 2020

Conversation

akinomaeni
Copy link
Contributor

@akinomaeni akinomaeni commented Apr 4, 2020

Summary

Add subsec to ActiveSupport::TimeWithZone#inspect.

before

Time.at(1498099140).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00 UTC +00:00"
Time.at(1498099140, 123456780, :nsec).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00 UTC +00:00"
Time.at(1498099140 + Rational("1/3")).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00 UTC +00:00"

after

Time.at(1498099140).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00.000000000 UTC +00:00"
Time.at(1498099140, 123456780, :nsec).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00.123456780 UTC +00:00"
Time.at(1498099140 + Rational("1/3")).in_time_zone.inspect
# => "Thu, 22 Jun 2017 02:39:00.333333333 UTC +00:00"

Other Information

Ruby 2.7 Time#inspect

It started inspecting sub-second of Time.
Rdoc Ticket Commit

$ docker run ruby:2.7.0-alpine ruby -e 'p Time.at(1498099140)'
2017-06-22 02:39:00 +0000
$ docker run ruby:2.7.0-alpine ruby -e 'p Time.at(1498099140, 123456780, :nsec)'
2017-06-22 02:39:00.12345678 +0000
$ docker run ruby:2.7.0-alpine ruby -e 'p Time.at(1498099140 + Rational("1/3"))'
2017-06-22 02:39:00 1/3 +0000
$ docker run ruby:2.6.5-alpine ruby -e 'p Time.at(1498099140)'
2017-06-22 02:39:00 +0000
$ docker run ruby:2.6.5-alpine ruby -e 'p Time.at(1498099140, 123456780, :nsec)'
2017-06-22 02:39:00 +0000
$ docker run ruby:2.6.5-alpine ruby -e 'p Time.at(1498099140 + Rational("1/3"))'
2017-06-22 02:39:00 +0000

Motivation

ActiveSupport::TimeWithZone#inspect does not display sub-second. And it confuses me that times having different sub-seconds look like the same time.

  • Database adapter: mysql2
  • create_table :songs { |t| t.datetime :released_at, precision: 6 }
# git branch => master
vagrant@rails-dev-box:/vagrant/my-test-app$ bin/rails c
Loading development environment (Rails 6.1.0.alpha)
irb(main):001:0> now = Time.current
=> Sat, 04 Apr 2020 13:37:58 UTC +00:00
irb(main):002:0> released_at = Song.new(released_at: now).released_at
=> Sat, 04 Apr 2020 13:37:58 UTC +00:00
irb(main):003:0> now == released_at
=> false
irb(main):004:0> now.nsec
=> 433841399
irb(main):005:0> released_at.nsec
=> 433841000

After the change it's clear.

# git branch => inspect-time-with-zone-subsec
vagrant@rails-dev-box:/vagrant/my-test-app$ bin/rails c
Loading development environment (Rails 6.1.0.alpha)
irb(main):001:0> now = Time.current
=> Sat, 04 Apr 2020 13:39:12.231596578 UTC +00:00
irb(main):002:0> released_at = Song.new(released_at: now).released_at
=> Sat, 04 Apr 2020 13:39:12.231596 UTC +00:00
irb(main):003:0> now == released_at
=> false

Time.at(1498099140).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00 UTC +00:00"
Time.at(1498099140, 123456780, :nsec).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00.12345678 UTC +00:00"
Time.at(1498099140 + Rational("1/3")).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00 1/3 UTC +00:00"
Copy link
Contributor

@kaspth kaspth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting idea! Hopefully this should make it more obvious when times are different in minitest assertions too.

@@ -139,7 +139,7 @@ def zone
#
# Time.zone.now.inspect # => "Thu, 04 Dec 2014 11:00:25 EST -05:00"
def inspect
"#{time.strftime('%a, %d %b %Y %H:%M:%S')} #{zone} #{formatted_offset}"
"#{time.strftime('%a, %d %b %Y %H:%M:%S')}#{formatted_subsec_with_delimiter} #{zone} #{formatted_offset}"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we get this via strftime in 2.7? Although it's potentially dangerous to make this Ruby version specific, there's a significant amount of code that it would be nice to trim away.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like %L and %N are candidates:

  %L - Millisecond of the second (000..999)
  %N - Fractional seconds digits, default is 9 digits (nanosecond)
          %3N  millisecond (3 digits)   %15N femtosecond (15 digits)
          %6N  microsecond (6 digits)   %18N attosecond  (18 digits)
          %9N  nanosecond  (9 digits)   %21N zeptosecond (21 digits)
          %12N picosecond (12 digits)   %24N yoctosecond (24 digits)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaspth Thank you for your suggestion.

What do you think about changing the handling of rational subsecs that cannot be represented by integers even if subsec digits is increased?


Ruby implements its own subsec format without using strftime in Time#inspect.
https://github.com/ruby/ruby/blob/d827c718db32c07f025f88fd792e9407e19e66e1/time.c#L4143-L4157
So we cannot get the same formatted subsec via strftime.

case with the current formatted_subsec_with_delimiter.

irb(main):001:0> t1 = Time.at(1498099140 + Rational("2/7")).in_time_zone
=> Thu, 22 Jun 2017 02:39:00 2/7 UTC +00:00
irb(main):002:0> t2 = Time.at(1498099140, 285714285, :nsec).in_time_zone
=> Thu, 22 Jun 2017 02:39:00.285714285 UTC +00:00
irb(main):003:0> t1 == t2
=> false

case with %9N

irb(main):001:0> t1 = Time.at(1498099140 + Rational("2/7")).in_time_zone
=> Thu, 22 Jun 2017 02:39:00.285714285 UTC +00:00
irb(main):002:0> t2 = Time.at(1498099140, 285714285, :nsec).in_time_zone
=> Thu, 22 Jun 2017 02:39:00.285714285 UTC +00:00
irb(main):003:0> t1 == t2
=> false

In my opinion, I want to know the contents exactly with inspect, different from to_s and other formatting methods.

But if Rails team thinks that maintainability is more important than accuracy here, I would support it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated doc 899d006

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaspth Here is the patch with %9N akinomaeni@7582ed7. Should I push it to this PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wow, that's cool! How does it look on Ruby 2.6? Feel free to push to here and get the build to show us.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

irb(main):001:0> RUBY_VERSION
=> "1.9.3"
irb(main):002:0> Time.now.strftime('%9N')
=> "437936000"

%9N is already available on Ruby 1.9. So no worry on 2.6.

The new thing in Ruby 2.7 is this change.

Ruby implements its own subsec format without using strftime in Time#inspect.
https://github.com/ruby/ruby/blob/d827c718db32c07f025f88fd792e9407e19e66e1/time.c#L4143-L4157

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

case with %9N

irb(main):001:0> t1 = Time.at(1498099140 + Rational("2/7")).in_time_zone
=> Thu, 22 Jun 2017 02:39:00.285714285 UTC +00:00
irb(main):002:0> t2 = Time.at(1498099140, 285714285, :nsec).in_time_zone
=> Thu, 22 Jun 2017 02:39:00.285714285 UTC +00:00
irb(main):003:0> t1 == t2
=> false

This is on Ruby 2.5.5.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kaspth Pushed!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new thing in Ruby 2.7 is this change.

Ah, I see what you mean. I think using %9N covers it just enough. The potential issue with Rational doesn't bother me with what you've shown me.

Time.at(1498099140).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00.000000000 UTC +00:00"
Time.at(1498099140, 123456780, :nsec).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00.123456780 UTC +00:00"
Time.at(1498099140 + Rational("1/3")).in_time_zone.inspect
=> "Thu, 22 Jun 2017 02:39:00.333333333 UTC +00:00"
@kaspth kaspth merged commit 4d09472 into rails:master Apr 11, 2020
@kaspth
Copy link
Contributor

kaspth commented Apr 11, 2020

Thanks for working on this! 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants